Add multi-server support for Jellyfin and AudiobookShelf#1491
Add multi-server support for Jellyfin and AudiobookShelf#1491matalvernaz wants to merge 2 commits intoTortugaPower:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds multi-server connection support for Jellyfin and AudiobookShelf so users can save, switch, and manage multiple server instances without re-entering credentials.
Changes:
- Persist multiple connections in Keychain (array), migrate from single-connection format, and track an “active” connection via UserDefaults.
- Introduce server picker views for the import (“Download from …”) flow when 2+ servers exist.
- Update Settings-connected views to list all servers with per-server sign-out and “Add Another Server” flow.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| BookPlayer/Jellyfin/Network/JellyfinConnectionService.swift | Stores/reloads multiple Jellyfin connections, manages active connection, and persists to Keychain/UserDefaults |
| BookPlayer/Jellyfin/Network/JellyfinConnectionData.swift | Adds id with backward-compatible decoding for migration |
| BookPlayer/Jellyfin/Connection Screen/JellyfinServerPickerView.swift | New server picker UI for multi-server import flow |
| BookPlayer/Jellyfin/Connection Screen/JellyfinConnectionViewModel.swift | Manages list of saved servers + activate/delete/add/cancel flows |
| BookPlayer/Jellyfin/Connection Screen/JellyfinConnectionView.swift | Routes between picker vs form based on mode/server count; adds Cancel toolbar |
| BookPlayer/Jellyfin/Connection Screen/JellyfinConnectedView.swift | Lists all servers in Settings and supports per-server actions |
| BookPlayer/AudiobookShelf/Network/AudiobookShelfConnectionService.swift | Stores/reloads multiple ABS connections, active selection, and Keychain persistence |
| BookPlayer/AudiobookShelf/Network/AudiobookShelfConnectionData.swift | Adds id with backward-compatible decoding for migration |
| BookPlayer/AudiobookShelf/Connection Screen/AudiobookShelfServerPickerView.swift | New server picker UI for multi-server import flow |
| BookPlayer/AudiobookShelf/Connection Screen/AudiobookShelfConnectionViewModel.swift | Manages list of saved servers + activate/delete/add/cancel flows |
| BookPlayer/AudiobookShelf/Connection Screen/AudiobookShelfConnectionView.swift | Routes between picker vs form based on mode/server count; adds Cancel toolbar |
| BookPlayer/AudiobookShelf/Connection Screen/AudiobookShelfConnectedView.swift | Lists all servers in Settings and supports per-server actions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private func reloadConnections() { | ||
| // Try new array format first | ||
| if let stored: [JellyfinConnectionData] = try? keychainService.get(.jellyfinConnection), | ||
| !stored.isEmpty { | ||
| connections = stored | ||
| } | ||
| // Migrate from old single-connection format (or array with one migrated item) | ||
| else if let stored: JellyfinConnectionData = try? keychainService.get(.jellyfinConnection) { | ||
| connections = [stored] | ||
| saveConnections() | ||
| } | ||
|
|
||
| private func isConnectionValid(_ data: JellyfinConnectionData) -> Bool { | ||
| return !data.userID.isEmpty && !data.accessToken.isEmpty | ||
| // Rebuild the client for the active connection | ||
| if let conn = connection { | ||
| client = createClient(serverUrlString: conn.url.absoluteString, accessToken: conn.accessToken) | ||
| if activeConnectionID == nil { | ||
| activeConnectionID = conn.id | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The previous implementation validated stored connection data before using it; this version loads connections without validation and can rebuild a client from invalid/empty credentials. Additionally, if activeConnectionID is set but no longer exists in connections, connection falls back to connections.first while activeConnectionID remains stale, which can lead to inconsistent persisted state. Suggested fix: filter loaded connections using isConnectionValid, and normalize activeConnectionID after loading (e.g., if it doesn’t match any connection, set it to connections.first?.id).
| if activeConnectionID == nil { | ||
| activeConnectionID = connections.first?.id |
There was a problem hiding this comment.
Like the Jellyfin service, this reload path doesn’t validate stored connections (even though isConnectionValid exists below) and can keep unusable credentials around. It also doesn’t handle the case where activeConnectionID is non-nil but no longer exists in connections (stale UserDefaults), leaving the persisted active ID inconsistent with the effective connection selection. Suggested fix: filter loaded connections via isConnectionValid, and if activeConnectionID doesn’t match any saved connection, reset it to connections.first?.id.
| if activeConnectionID == nil { | |
| activeConnectionID = connections.first?.id | |
| // Remove any invalid or incomplete connections | |
| connections = connections.filter { isConnectionValid($0) } | |
| // Ensure the active connection ID matches a saved connection (or reset it) | |
| if connections.isEmpty { | |
| activeConnectionID = nil | |
| } else if let activeID = activeConnectionID, | |
| !connections.contains(where: { $0.id == activeID }) { | |
| activeConnectionID = connections.first?.id | |
| } else if activeConnectionID == nil { | |
| activeConnectionID = connections.first?.id |
| Button { | ||
| viewModel.handleAddServerAction() | ||
| } label: { | ||
| Label("Add Another Server", systemImage: "plus.circle") |
There was a problem hiding this comment.
This introduces a new user-facing string as a hardcoded literal. The PR description notes this needs a localization key; please replace the literal with a localized string key (e.g., the suggested integration_add_server_button) for consistency with the rest of the integration UI.
| Label("Add Another Server", systemImage: "plus.circle") | |
| Label("integration_add_server_button".localized, systemImage: "plus.circle") |
| Button { | ||
| viewModel.handleAddServerAction() | ||
| } label: { | ||
| Label("Add Another Server", systemImage: "plus.circle") |
There was a problem hiding this comment.
Several texts appear to be missing .localized in the AudiobookShelf UI (e.g., integration_username_placeholder, logout_title, integration_section_login), which will cause the raw localization keys to be displayed to users. Please apply .localized consistently (matching the Jellyfin screens) and localize the new "Add Another Server" label via a localization key.
|
|
||
| // Username row | ||
| HStack { | ||
| Text("integration_username_placeholder") |
There was a problem hiding this comment.
Several texts appear to be missing .localized in the AudiobookShelf UI (e.g., integration_username_placeholder, logout_title, integration_section_login), which will cause the raw localization keys to be displayed to users. Please apply .localized consistently (matching the Jellyfin screens) and localize the new "Add Another Server" label via a localization key.
| } | ||
|
|
||
| // Sign out button | ||
| Button("logout_title", role: .destructive) { |
There was a problem hiding this comment.
Several texts appear to be missing .localized in the AudiobookShelf UI (e.g., integration_username_placeholder, logout_title, integration_section_login), which will cause the raw localization keys to be displayed to users. Please apply .localized consistently (matching the Jellyfin screens) and localize the new "Add Another Server" label via a localization key.
| .foregroundStyle(.red) | ||
| } header: { | ||
| if isActive { | ||
| Text("integration_section_login") |
There was a problem hiding this comment.
Several texts appear to be missing .localized in the AudiobookShelf UI (e.g., integration_username_placeholder, logout_title, integration_section_login), which will cause the raw localization keys to be displayed to users. Please apply .localized consistently (matching the Jellyfin screens) and localize the new "Add Another Server" label via a localization key.
| } | ||
| } | ||
| } header: { | ||
| Text("integration_section_login") |
There was a problem hiding this comment.
The server picker header uses a localization key without calling .localized, so it will likely render the key string itself. Update it to use the localized value (consistent with JellyfinServerPickerView).
| Text("integration_section_login") | |
| Text("integration_section_login".localized) |
| private var activeConnectionID: String? = UserDefaults.standard.string(forKey: "jellyfinActiveConnectionID") { | ||
| didSet { UserDefaults.standard.set(activeConnectionID, forKey: "jellyfinActiveConnectionID") } | ||
| } |
There was a problem hiding this comment.
The UserDefaults key is a string literal embedded in the service. To reduce the risk of typos and ease future refactors, consider centralizing these keys (e.g., a UserDefaultsKeys namespace/enum) and reusing it in both Jellyfin and AudiobookShelf services.
|
Addressed all review feedback:
|
|
@matalvernaz after the PR that adds the different options for AudiobookShelf, I reworked how both integrations are built, and the UX for how we handle showing the connection details. I made the last change also thinking of multiple connections for the integrations, but I still have a couple of things to work on prior to the next release, I'll put all the latest changes in the beta though |
|
Hey, thanks for the update. I see some of your changes and would be happy to keep working on this based on them. Is that something you'd want me to help with, or is there anything else that would be more useful? |
|
@matalvernaz hmmm I guess it's more towards what would be more useful to you, and what you want to see next in BookPlayer, multi-server support is something that I think we should do, but if you would rather do another thing, we could take a look at that too 👌 |
|
Okay awesome, I'll work on that then! I see how I'd need to change what I did. I can't think of anything else that'd be that useful yet, but will contribute when I do, and hopefully fix things to be better instead of worse. |
matalvernaz
force-pushed
the
feat/multi-server
branch
from
5745d22 to
b201d99
Compare
Adapts the multi-server feature to the protocol-based integration framework introduced in the integrations rework. Instead of integration-specific views and services, multi-server support now plugs into the shared IntegrationConnectionViewModelProtocol and generic views in MediaServerIntegration/. Data model (both integrations): - Add id: String (UUID) to connection data with backward-compatible Decodable init for zero-friction migration from single-connection format Connection services (both integrations): - Keychain storage: single object -> [ConnectionData] array - reloadConnections(): tries array format first, migrates old single object on first launch, validates via isConnectionValid, normalizes activeConnectionID - signIn(): appends new connection, deduplicates on url + userID - activateConnection(id:): switch active server - deleteConnection(id:): remove specific server Shared protocol & views: - IntegrationConnectionViewModelProtocol gains servers, isAddingServer, handleSignOutAction(id:), handleActivateAction(id:), handleAddServerAction(), handleCancelAddServerAction() - IntegrationConnectedView: shows all saved servers with per-server sign-out when 2+, plus "Add Another Server" button - IntegrationConnectionView: handles isAddingServer flow with Cancel toolbar button and full connect/sign-in progression - New IntegrationServerPickerView: shared picker for import flow when 2+ servers exist Root views (both integrations): - Show server picker sheet when 2+ servers on launch - Reload library when switching servers
matalvernaz
force-pushed
the
feat/multi-server
branch
from
b201d99 to
a161345
Compare
Replace the separate "Download from Jellyfin" and "Download from AudiobookShelf" menu items with one "Media Servers" button that opens a unified view showing all saved servers from both integrations. - Add MediaServersView with unified server list, type picker for adding new servers, and in-place add-server sheets - Add skipServerPicker parameter to root views so the caller can bypass the per-integration server picker after activating a server - Add .mediaServers case to IntegrationSheet enum - Add localization keys for the new UI Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Hey! I've rebased this onto the latest develop and added a unified "Media Servers" UI on top of the multi-server work. What changed: The two separate "Download from Jellyfin" / "Download from AudiobookShelf" menu items are now replaced with a single "Media Servers" button. It opens a unified list showing all saved servers from both integrations (with type icons), and an "Add Server" button that lets you pick the server type and go through the connection flow. I've finally managed to get hold of a macOS compilation environment, so I can test on my phone. So far I've only tested with two Jellyfin servers added, but the multi-server support seems to be working alright for me. Will continue testing with AudiobookShelf as well. The branch is up to date with develop as of today. |
Problem
Both integrations only support a single saved server. Users with multiple Jellyfin or AudiobookShelf instances (e.g. home + work, personal + shared) have to sign out and re-enter credentials every time they want to switch.
Solution
Store any number of server connections and expose a unified "Media Servers" experience that combines both Jellyfin and AudiobookShelf into one entry point.
"Media Servers" (the import sheet, replaces the separate "Download from…" buttons):
Settings screen (connection details, accessed from within each library browser):
Changes
Data model (both integrations)
id: String(UUID) toJellyfinConnectionData/AudiobookShelfConnectionDataDecodableinit generates a UUID for any existing saved data that lacks the field — zero-friction migrationConnection services (both integrations)
[ConnectionData]arrayreloadConnections(): tries array format first, migrates old single-object format on first launchsignIn(): appends new connection, deduplicates onurl + userIDactivateConnection(id:): switch active serverdeleteConnection(id:): remove specific server;deleteConnection()is kept for backward compatibilityactiveConnectionIDpersisted inUserDefaultsNew views
MediaServersView— unified server list combining Jellyfin and AudiobookShelf servers, with type icons, add-server flow, and in-place add sheetsIntegrationServerPickerView— per-integration server picker (used internally by root views whenskipServerPickeris false)Updated views
ItemListView— replaced separate "Download from Jellyfin" / "Download from AudiobookShelf" menu buttons with a single "Media Servers" buttonMainView— handles the new.mediaServerssheet case; passesskipServerPicker: trueto root views when launched from the unified listListStateManager— added.mediaServerscase toIntegrationSheetenumJellyfinRootView/AudiobookShelfRootView— addedskipServerPickerparameter to bypass the per-integration server picker when the caller has already activated the desired serverIntegrationConnectedView— renders all saved servers as a list (used in Settings)IntegrationConnectionView— routes to picker vs. form based on server count and view mode; Cancel toolbar button when adding a second server from SettingsView models (both integrations)
handleSignOutAction(id:)— remove a specific serverhandleActivateAction(id:)— switch active server and navigate to its libraryhandleAddServerAction()/handleCancelAddServerAction()— Settings-only add flowTesting
No servers (fresh start):
Single server (regression):
Multiple servers (mixed types):
Multiple servers (same type):
Migration:
Notes