Add multi-server support for Jellyfin and AudiobookShelf#1491
Add multi-server support for Jellyfin and AudiobookShelf#1491matalvernaz wants to merge 1 commit into
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
2 times, most recently
from
b201d99 to
a161345
Compare
|
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. |
Brings the branch up to date with develop (Core Data v11 migration, custom-headers PR TortugaPower#1513, sticky-sort, end-of-chapter fix, etc.) and folds in a few iterations on top: - A single \"Media Servers\" entry in the import menu in place of the separate Jellyfin / AudiobookShelf items. Lists every saved server from both integrations with type icons; \"Add Server\" picks the type and goes through the connection flow. - Tap-a-server presents the per-integration library browser as a sheet on top of the unified list, with a leading \"Media Servers\" back button that closes the inner sheet and lands you back on the list. Earlier rework tried to push via NavigationStack but pushing a TabView in a NavigationStack auto-pops on iOS 26 — sheet-on-sheet sidesteps that entirely. - Library picker no longer traps you when there are 2+ libraries and none chosen yet (Cancel is unconditional and falls back to dismissing the whole sheet when there's no library to dismiss to). - Connection-form xmark closes the form instead of dismissing the whole integration view. - Empty-state rows restyled to match the populated server-row layout so they read as tappable. Brand strings consolidated through ServerType.displayName, fonts unified on bpFont.
|
Hey! Got this caught up to current develop (lots to fold in: Core Data v11, custom-headers from #1513, sticky-sort, EoC fix). Rolled in a few iterations on the multi-server UI on top. Tapping a server now opens its library browser as a sheet on top of the unified list, with a "Media Servers" back button in the leading slot. I tried pushing via NavigationStack first, but pushing a TabView inside one auto-pops on iOS 26 — sheet-on-sheet just sidesteps that. Also fixed a couple of dead-end exits I noticed while testing. Library picker now has an unconditional Cancel (falls back to dismissing the integration sheet if you haven't picked a library yet), and the connection-form xmark closes just the form instead of bailing out of the whole integration view. The empty state's add-server rows got restyled to match the populated server-row layout (icon + name + chevron) so they read as tappable. Tested on TestFlight build 5.20.0 / 20260505091947 with two Jellyfin servers and one ABS. Happy to split anything out if it's easier to review separately. |
matalvernaz
force-pushed
the
feat/multi-server
branch
from
a85e9c1 to
e21974a
Compare
|
thanks @matalvernaz ! I had a mental block on getting out the current release 5.20.0 before looking into other PRs, but now that's done, I can focus again |
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