mirror of
https://codeberg.org/secana/Forji.git
synced 2026-06-16 05:13:55 -07:00
fix: key multi-instance fallback by account (#29)
## Summary This changes the multi-instance fallback map to key bootstrapped connections by account instead of server URL only. Previously, `connect(instances:)` built a dictionary keyed only by `instance.serverURL`. If two accounts used the same Forgejo instance, `Dictionary(uniqueKeysWithValues:)` could trap on duplicate keys, and fallback lookup could also reuse the wrong account for a failed restore. ## Changes - Add an account key based on normalized server URL + username. - Use that key for bootstrapped connection fallback in `MultiInstanceManager`. - Add a regression test covering two token-auth accounts on the same server URL. ## Verification - `git diff --check` passes. - I could not run `xcodebuild` locally because the available Xcode install has not accepted the license on this machine (`xcodebuild` exits before build/test execution). Co-authored-by: Piotr Durlej <pdurlej@users.noreply.github.com> Reviewed-on: https://codeberg.org/secana/Forji/pulls/29 Reviewed-by: secana <secana@noreply.codeberg.org>
This commit is contained in:
parent
f7eba701e2
commit
0b815335a9
2 changed files with 68 additions and 2 deletions
|
|
@ -52,7 +52,7 @@ final class MultiInstanceManager {
|
|||
return await group.reduce(into: [(Int, Bool, String?)]()) { $0.append($1) }
|
||||
}
|
||||
|
||||
let bootstrapped = Dictionary(uniqueKeysWithValues: connections.map { ($0.instance.serverURL, $0) })
|
||||
let bootstrapped = bootstrappedConnectionsByAccountKey()
|
||||
var newConnections: [(instance: ForgejoInstance, authService: AuthenticationService)] = []
|
||||
var newFailed: [(instance: ForgejoInstance, error: String)] = []
|
||||
|
||||
|
|
@ -60,7 +60,7 @@ final class MultiInstanceManager {
|
|||
let instance = instances[index]
|
||||
if success {
|
||||
newConnections.append((instance: instance, authService: authServices[index]))
|
||||
} else if let existing = bootstrapped[instance.serverURL] {
|
||||
} else if let existing = bootstrapped[accountKey(for: instance)] {
|
||||
newConnections.append(existing)
|
||||
} else {
|
||||
newFailed.append((instance: instance, error: error ?? "Unknown error"))
|
||||
|
|
@ -115,6 +115,20 @@ final class MultiInstanceManager {
|
|||
func displayName(for instance: ForgejoInstance) -> String {
|
||||
instance.name.isEmpty ? instance.serverURL : instance.name
|
||||
}
|
||||
|
||||
func accountKey(for instance: ForgejoInstance) -> String {
|
||||
"\(ForgejoClient.normalizeServerURL(instance.serverURL)):\(instance.username)"
|
||||
}
|
||||
|
||||
func bootstrappedConnectionsByAccountKey()
|
||||
-> [String: (instance: ForgejoInstance, authService: AuthenticationService)]
|
||||
{
|
||||
var bootstrapped: [String: (instance: ForgejoInstance, authService: AuthenticationService)] = [:]
|
||||
for connection in connections {
|
||||
bootstrapped[accountKey(for: connection.instance)] = connection
|
||||
}
|
||||
return bootstrapped
|
||||
}
|
||||
}
|
||||
|
||||
/// Sendable snapshot of ForgejoInstance data needed for session restore.
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import ForgejoKit
|
||||
import Foundation
|
||||
import Testing
|
||||
@testable import Forji
|
||||
|
||||
|
|
@ -68,4 +69,55 @@ struct MultiInstanceManagerTests {
|
|||
let sources = manager.connectionSources()
|
||||
#expect(sources.isEmpty)
|
||||
}
|
||||
|
||||
@Test func bootstrappedConnectionMapKeepsAccountsWithSameServerURL() async throws {
|
||||
let serverURL = "https://forgejo.example.com/"
|
||||
let normalizedServerURL = ForgejoClient.normalizeServerURL(serverURL)
|
||||
let suffix = UUID().uuidString
|
||||
let firstUsername = "alice-\(suffix)"
|
||||
let secondUsername = "bob-\(suffix)"
|
||||
let first = ForgejoInstance(
|
||||
serverURL: serverURL,
|
||||
username: firstUsername,
|
||||
useTokenAuth: true,
|
||||
)
|
||||
let second = ForgejoInstance(
|
||||
serverURL: serverURL,
|
||||
username: secondUsername,
|
||||
useTokenAuth: true,
|
||||
)
|
||||
|
||||
try await KeychainManager.shared.saveToken(
|
||||
"token-\(firstUsername)",
|
||||
for: normalizedServerURL,
|
||||
username: firstUsername,
|
||||
)
|
||||
try await KeychainManager.shared.saveToken(
|
||||
"token-\(secondUsername)",
|
||||
for: normalizedServerURL,
|
||||
username: secondUsername,
|
||||
)
|
||||
|
||||
let manager = MultiInstanceManager()
|
||||
manager.bootstrap(instances: [first, second])
|
||||
let bootstrapped = manager.bootstrappedConnectionsByAccountKey()
|
||||
|
||||
#expect(manager.connections.count == 2)
|
||||
#expect(bootstrapped.count == 2)
|
||||
#expect(
|
||||
bootstrapped[manager.accountKey(for: first)]?.instance.username == firstUsername
|
||||
)
|
||||
#expect(
|
||||
bootstrapped[manager.accountKey(for: second)]?.instance.username == secondUsername
|
||||
)
|
||||
|
||||
try await KeychainManager.shared.deleteToken(
|
||||
for: normalizedServerURL,
|
||||
username: firstUsername,
|
||||
)
|
||||
try await KeychainManager.shared.deleteToken(
|
||||
for: normalizedServerURL,
|
||||
username: secondUsername,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue