dallasgroot/Forji
1
0
Fork 0
mirror of https://codeberg.org/secana/Forji.git synced 2026-06-16 05:13:55 -07:00
Code Issues Projects Releases Packages Wiki Activity Actions

refactor: small changes

Browse source
This commit is contained in:
Stefan Hausotte 2026-03-21 15:23:44 +01:00
parent 10bbed5596
commit 0bcca64d3a
8 changed files with 145 additions and 84 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
Forji/Forji/Helpers/TaggedItem.swift
View file

@ -6,8 +6,8 @@ struct TaggedItem<T: Identifiable>: Identifiable {
let instanceName: String
let authService: AuthenticationService
init(item: T, instanceName: String, authService: AuthenticationService) {
id = "\(instanceName):\(item.id)"
init(item: T, sourceKey: String, instanceName: String, authService: AuthenticationService) {
id = "\(sourceKey):\(item.id)"
self.item = item
self.instanceName = instanceName
self.authService = authService
@ -22,7 +22,10 @@ struct TaggedItem<T: Identifiable>: Identifiable {
for (index, items) in batches {
let source = sources[index]
for item in items {
let tagged = TaggedItem(item: item, instanceName: source.name, authService: source.authService)
let tagged = TaggedItem(
item: item, sourceKey: source.sourceKey,
instanceName: source.name, authService: source.authService,
)
if seen.insert(tagged.id).inserted {
merged.append(tagged)
}

19
Forji/Forji/Services/MultiInstanceManager.swift
View file

@ -18,7 +18,6 @@ final class MultiInstanceManager {
connections = []
failedInstances = []
// Extract Sendable data before entering the task group
let snapshots = instances.map { instance in
InstanceSnapshot(
serverURL: instance.serverURL,
@ -28,7 +27,6 @@ final class MultiInstanceManager {
)
}
// Create AuthenticationService instances on the main actor before the task group
let authServices = snapshots.map { _ in AuthenticationService() }
let results = await withTaskGroup(
@ -94,8 +92,10 @@ final class MultiInstanceManager {
func connectionSources() -> [ConnectionSource] {
connections.compactMap { connection in
guard let client = connection.authService.client else { return nil }
let instance = connection.instance
return ConnectionSource(
name: displayName(for: connection.instance),
sourceKey: "\(instance.serverURL):\(instance.username)",
name: displayName(for: instance),
client: client,
authService: connection.authService,
)
@ -116,7 +116,20 @@ struct InstanceSnapshot: Sendable {
}
struct ConnectionSource {
let sourceKey: String
let name: String
let client: ForgejoClient
let authService: AuthenticationService
}
/// Runs a fetch, returning empty on timeout or cancellation.
/// All other errors propagate so callers can surface them.
func resilientFetch<T>(_ fetch: () async throws -> [T]) async throws -> [T] {
do {
return try await fetch()
} catch is CancellationError {
return []
} catch let urlError as URLError where urlError.code == .timedOut || urlError.code == .cancelled {
return []
}
}

17
Forji/Forji/Views/InstanceFormView.swift
View file

@ -235,8 +235,25 @@ struct InstanceFormView: View {
}
}
private var editingInstanceId: String? {
if case let .edit(instance) = mode { return instance.serverURL + instance.username }
return nil
}
// swiftlint:disable:next function_body_length cyclomatic_complexity
private func handleSave() {
if !name.isEmpty {
let conflict = instances.first { inst in
let instId = inst.serverURL + inst.username
return inst.name == name && instId != editingInstanceId
}
if conflict != nil {
errorMessage = "An instance named \"\(name)\" already exists. Please choose a different name."
showError = true
return
}
}
isLoading = true
errorMessage = nil

25
Forji/Forji/Views/MergedIssuesOverviewView.swift
View file

@ -156,26 +156,27 @@ struct MergedIssuesOverviewView: View {
for (index, source) in sources.enumerated() {
let client = source.client
if query.isEmpty {
// Fire separate requests per involvement flag for OR-union semantics
for flag in InvolvementScope.issueFlags {
group.addTask {
let service = IssueService(client: client)
let issues = (try? await service.searchIssues(
type: "issues", state: state, page: page, limit: limit,
assigned: flag == .assigned, created: flag == .created,
mentioned: flag == .mentioned
)) ?? []
return (index, issues)
return (index, try await resilientFetch {
try await service.searchIssues(
type: "issues", state: state, page: page, limit: limit,
assigned: flag == .assigned, created: flag == .created,
mentioned: flag == .mentioned
)
})
}
}
} else {
group.addTask {
let service = IssueService(client: client)
let issues = (try? await service.searchIssues(
type: "issues", state: state, query: query,
page: page, limit: limit
)) ?? []
return (index, issues)
return (index, try await resilientFetch {
try await service.searchIssues(
type: "issues", state: state, query: query,
page: page, limit: limit
)
})
}
}
}

9
Forji/Forji/Views/MergedNotificationsOverviewView.swift
View file

@ -144,10 +144,11 @@ struct MergedNotificationsOverviewView: View {
let client = source.client
group.addTask {
let service = NotificationService(client: client)
let notifications = (try? await service.fetchNotifications(
statusTypes: types, page: page, limit: limit
)) ?? []
return (index, notifications)
return (index, try await resilientFetch {
try await service.fetchNotifications(
statusTypes: types, page: page, limit: limit
)
})
}
}
return try await group.reduce(into: [(Int, [NotificationThread])]()) { $0.append($1) }

24
Forji/Forji/Views/MergedPullRequestsOverviewView.swift
View file

@ -156,22 +156,24 @@ struct MergedPullRequestsOverviewView: View {
for flag in InvolvementScope.pullRequestFlags {
group.addTask {
let service = IssueService(client: client)
let issues = (try? await service.searchIssues(
type: "pulls", state: state, page: page, limit: limit,
assigned: flag == .assigned, created: flag == .created,
mentioned: flag == .mentioned, reviewRequested: flag == .reviewRequested
)) ?? []
return (index, issues)
return (index, try await resilientFetch {
try await service.searchIssues(
type: "pulls", state: state, page: page, limit: limit,
assigned: flag == .assigned, created: flag == .created,
mentioned: flag == .mentioned, reviewRequested: flag == .reviewRequested
)
})
}
}
} else {
group.addTask {
let service = IssueService(client: client)
let issues = (try? await service.searchIssues(
type: "pulls", state: state, query: query,
page: page, limit: limit
)) ?? []
return (index, issues)
return (index, try await resilientFetch {
try await service.searchIssues(
type: "pulls", state: state, query: query,
page: page, limit: limit
)
})
}
}
}

31
Forji/Forji/Views/MergedRepositoryListView.swift
View file

@ -93,9 +93,9 @@ struct MergedRepositoryListView: View {
errorMessage = nil
do {
let items = try await fetchFromAllInstances(page: 1, limit: pageSize)
let (items, anyFull) = try await fetchFromAllInstances(page: 1, limit: pageSize)
repositories = items
hasMore = items.count >= pageSize * manager.connections.count
hasMore = anyFull
currentPage = 2
hasLoaded = true
} catch is CancellationError {
@ -113,9 +113,9 @@ struct MergedRepositoryListView: View {
isLoading = true
do {
let items = try await fetchFromAllInstances(page: currentPage, limit: pageSize)
let (items, anyFull) = try await fetchFromAllInstances(page: currentPage, limit: pageSize)
repositories.append(contentsOf: items)
hasMore = items.count >= pageSize
hasMore = anyFull
currentPage += 1
} catch is CancellationError {
// Ignore
@ -127,9 +127,10 @@ struct MergedRepositoryListView: View {
isLoading = false
}
private func fetchFromAllInstances(page: Int, limit: Int) async throws -> [TaggedItem<Repository>] {
private func fetchFromAllInstances(
page: Int, limit: Int
) async throws -> (items: [TaggedItem<Repository>], anySourceFull: Bool) {
let query = searchText
let sources = manager.connectionSources()
let batches = try await withThrowingTaskGroup(of: (Int, [Repository]).self) { group in
@ -137,20 +138,22 @@ struct MergedRepositoryListView: View {
let client = source.client
group.addTask {
let service = RepositoryService(client: client)
let repos: [Repository]
if query.isEmpty {
repos = (try? await service.fetchUserRepositories(page: page, limit: limit)) ?? []
} else {
repos = (try? await service.searchRepositories(query: query, page: page, limit: limit)) ?? []
}
return (index, repos)
return (index, try await resilientFetch {
if query.isEmpty {
return try await service.fetchUserRepositories(page: page, limit: limit)
} else {
return try await service.searchRepositories(query: query, page: page, limit: limit)
}
})
}
}
return try await group.reduce(into: [(Int, [Repository])]()) { $0.append($1) }
}
return TaggedItem.mergeAndDeduplicate(batches: batches, sources: sources)
let anySourceFull = batches.contains { $0.1.count >= limit }
let merged = TaggedItem.mergeAndDeduplicate(batches: batches, sources: sources)
.sorted { ($0.item.updatedAt ?? .distantPast) > ($1.item.updatedAt ?? .distantPast) }
return (merged, anySourceFull)
}
}

95
Forji/ForjiTests/TaggedItemTests.swift
View file

@ -19,41 +19,76 @@ struct TaggedItemTests {
)
}
// MARK: - ID Generation
@Test func idCombinesInstanceNameAndItemId() {
let auth = makeAuth()
let issue = makeIssue(id: 42)
let tagged = TaggedItem(item: issue, instanceName: "work", authService: auth)
#expect(tagged.id == "work:42")
private func makeSource(
key: String = "https://example.com:user",
name: String = "Example",
) -> ConnectionSource {
ConnectionSource(
sourceKey: key,
name: name,
client: ForgejoClient(serverURL: "https://example.com", username: "user", token: "t"),
authService: makeAuth()
)
}
@Test func sameItemDifferentInstancesHaveDifferentIds() {
// MARK: - ID Generation
@Test func idUsesSourceKeyNotDisplayName() {
let auth = makeAuth()
let issue = makeIssue(id: 42)
let tagged = TaggedItem(
item: issue, sourceKey: "https://work.com:admin",
instanceName: "Work", authService: auth
)
#expect(tagged.id == "https://work.com:admin:42")
#expect(tagged.instanceName == "Work")
}
@Test func sameItemDifferentSourceKeysHaveDifferentIds() {
let auth = makeAuth()
let issue = makeIssue(id: 1)
let taggedA = TaggedItem(item: issue, instanceName: "work", authService: auth)
let taggedB = TaggedItem(item: issue, instanceName: "personal", authService: auth)
let taggedA = TaggedItem(
item: issue, sourceKey: "https://a.com:user",
instanceName: "A", authService: auth
)
let taggedB = TaggedItem(
item: issue, sourceKey: "https://b.com:user",
instanceName: "B", authService: auth
)
#expect(taggedA.id != taggedB.id)
}
@Test func sameInstanceSameItemIdProducesSameId() {
@Test func sameDisplayNameDifferentSourceKeysNeverCollide() {
let auth = makeAuth()
let issue = makeIssue(id: 1)
let taggedA = TaggedItem(
item: issue, sourceKey: "https://a.com:user",
instanceName: "Same Name", authService: auth
)
let taggedB = TaggedItem(
item: issue, sourceKey: "https://b.com:user",
instanceName: "Same Name", authService: auth
)
#expect(taggedA.id != taggedB.id)
}
@Test func sameSourceKeySameItemIdProducesSameId() {
let auth = makeAuth()
let issueA = makeIssue(id: 5)
let issueB = makeIssue(id: 5, title: "Different title")
let taggedA = TaggedItem(item: issueA, instanceName: "srv", authService: auth)
let taggedB = TaggedItem(item: issueB, instanceName: "srv", authService: auth)
let taggedA = TaggedItem(
item: issueA, sourceKey: "srv", instanceName: "srv", authService: auth
)
let taggedB = TaggedItem(
item: issueB, sourceKey: "srv", instanceName: "srv", authService: auth
)
#expect(taggedA.id == taggedB.id)
}
// MARK: - Merge and Deduplicate
@Test func mergeAndDeduplicateRemovesDuplicateIds() {
let auth = makeAuth()
let source = ConnectionSource(
name: "srv",
client: ForgejoClient(serverURL: "https://example.com", username: "u", token: "t"),
authService: auth
)
let source = makeSource()
let issue = makeIssue(id: 1)
let batches: [(Int, [FIssue])] = [(0, [issue]), (0, [issue])]
@ -62,17 +97,8 @@ struct TaggedItemTests {
}
@Test func mergeAndDeduplicateKeepsItemsFromDifferentSources() {
let auth = makeAuth()
let sourceA = ConnectionSource(
name: "work",
client: ForgejoClient(serverURL: "https://work.example.com", username: "u", token: "t"),
authService: auth
)
let sourceB = ConnectionSource(
name: "personal",
client: ForgejoClient(serverURL: "https://personal.example.com", username: "u", token: "t"),
authService: auth
)
let sourceA = makeSource(key: "https://work.com:u", name: "work")
let sourceB = makeSource(key: "https://personal.com:u", name: "personal")
let issue = makeIssue(id: 1)
let batches: [(Int, [FIssue])] = [(0, [issue]), (1, [issue])]
@ -81,12 +107,7 @@ struct TaggedItemTests {
}
@Test func mergeAndDeduplicatePreservesAllFields() {
let auth = makeAuth()
let source = ConnectionSource(
name: "myserver",
client: ForgejoClient(serverURL: "https://example.com", username: "u", token: "t"),
authService: auth
)
let source = makeSource(key: "https://example.com:u", name: "myserver")
let issue = makeIssue(id: 7, title: "Important")
let batches: [(Int, [FIssue])] = [(0, [issue])]
@ -94,7 +115,7 @@ struct TaggedItemTests {
#expect(result.count == 1)
#expect(result[0].item.title == "Important")
#expect(result[0].instanceName == "myserver")
#expect(result[0].id == "myserver:7")
#expect(result[0].id == "https://example.com:u:7")
}
@Test func mergeAndDeduplicateReturnsEmptyForEmptyBatches() {