Add multi-server support for Jellyfin and AudiobookShelf#1491
Conversation
|
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 |
activateConnection and deleteConnection were calling createClient without passing the active connection's customHeaders, so any server behind a Cloudflare Access (or similar) proxy stopped responding after the user switched servers or signed out of one of several — the next request went out missing the required CF-Access-Client-* headers and was rejected until the app was relaunched (the only path that did pass them was reloadConnections at launch). Centralize the client-rebuild in rebuildClient(for:) so future call sites can't drop headers, and capture the old client before mutating state so the fire-and-forget signOut doesn't race the new client assignment.
When fetchLibraries (or the equivalent ABS call) threw, the resulting alert had a single OK button that set showConnectionForm = true. That flow is the right one for "the user wants to sign in again" but it's the wrong one for everything else (transient network, expired token, custom-header proxy hiccup). Users who hit it would re-add their server and end up with a duplicate because the byte-exact URL dedup in the connection service didn't treat trailing-slash / scheme variants as the same connection. Replace the single OK with three explicit buttons — Sign In (the old behavior, now opt-in), Retry (re-call loadLibraries), and Cancel (close the integration sheet, return to the Media Servers list). Same fix on both JellyfinRootView and AudiobookShelfRootView. New "integration_retry_button" localization key seeded with English literals across all 26 .lproj files matching the rest of the fork's localization pattern.
IntegrationConnectionView's onConnect / onSignIn / onStartQuickConnect each kicked off an unstructured Task that the view had no handle to. If the user dismissed the sheet (swipe-to-dismiss or hitting the close button) while a sign-in was in flight, the Task kept running and could persist a connection to the service after the user thought they gave up on the operation. Store each action's Task in @State and cancel it from .onDisappear. Catch CancellationError separately so the dismissal doesn't get surfaced as an alert. The View-side cancel alone isn't sufficient — Swift cooperative cancellation only flags the Task, it doesn't synchronously interrupt an awaiting call, and the service methods kept persisting after the network call returned. Add try Task.checkCancellation() inside JellyfinConnectionService.signIn(username:password:...) and AudiobookShelfConnectionService.signIn(...) immediately after the auth round-trip returns, so the persist step is skipped on cancel. The matching call inside signInWithQuickConnect was already added in the Quick-Connect cancel-race fix.
Two small input-validation gaps:
H18 — pingServer accepts a schemeless string like "abs.example.com"
verbatim because URL(string:) parses it as a relative path. The
subsequent appendingPathComponent("ping") yields "abs.example.com/ping"
which URLSession returns an opaque .unsupportedURL for, and the user
has no idea they needed to prepend https://. Normalize in the view
model before calling pingServer: trim whitespace, prepend https:// if
no scheme. Mirror the result back into the form field so the user can
see what we actually used.
H19 — Username and password were passed to ABS auth verbatim, so an
autocorrect-inserted trailing space on the username is enough to fail
otherwise-correct credentials with a 401. Trim both in
handleSignInAction.
Foundation URL equality is byte-exact, so https://server vs https://server/ vs https://server:443 vs https://Server were treated as four different connections by the deduplication in JellyfinConnectionService and AudiobookShelfConnectionService. Users hitting the C2 error alert in either integration would commonly re-add their server slightly differently (often with a trailing slash) and end up with two saved entries, one of which was broken. Add URL.canonicalDedupKey to BookPlayerKit's URL extension: lowercase scheme/host, drop default ports (80/443), strip the trailing slash on non-root paths, and drop userinfo/query/fragment. Both services' removeAll-then-append dedup blocks now compare canonical keys.
Token-expiry / mid-session-unauthorized had no first-class handling —
401/403 propagated as a generic load failure, the alert offered a
single OK that pushed the user into the add-server form, and users
who took it ended up with duplicate connections (the C2-minimum fix
just rearranged the alert buttons to stop the worst case).
Distinguish session-expired from generic failures end to end:
- New IntegrationError.sessionExpired(serverName:) carries the offending
server name so the alert message is specific ("Your session for
Library Server expired. Sign in again to continue.").
- Jellyfin: wrap the api-client `send<T>` to catch
APIError.unacceptableStatusCode(401|403) and throw .sessionExpired —
only when there's actually a saved connection (so pre-sign-in probes
inside `findServer` aren't mis-classified).
- ABS: centralize HTTP status validation in
validateAuthenticatedResponse(_:) and use it across every
authenticated data-fetch site (fetchLibraries, fetchItems,
fetchItemDetails, search, …). Same connection-presence guard. The
signIn path keeps its existing 401 → URLError(.userAuthenticationRequired)
mapping ("wrong password" is a different UX than "your session expired").
- Both RootView alerts (JellyfinRootView, AudiobookShelfRootView)
special-case `IntegrationError.isSessionExpired`: show Sign In +
Cancel only, skip the Retry button that's meaningless when the
token's revoked.
- Both signIn implementations now preserve the existing connection's
id and selectedLibraryId when re-authing on the same
(canonical-url, userID) pair — so when the user signs in again
they land back on the same library context they had before,
not a fresh record. This is the half of the fix that makes "Sign In
again" actually transparent UX rather than "you lost your saved
library choice".
New localization key `integration_error_session_expired` seeded with
English literals across all 26 .lproj files.
|
Hey @GianniCarlo, thanks for jumping on this. Copilot caught a lot of locale-file noise, but there are three things buried in there that look like real regressions from
Happy to push the fixes myself if that's easier, just let me know. The locale-key seeds (missing |
|
Hi @matalvernaz ! thanks for pointing those out, I'll take care of them, my intend is to merge this PR today, also I have yet to respond to Alex's email, I'll get to that this weekend 👌 |
| if let port = components.port, | ||
| (components.scheme == "http" && port == 80) | ||
| || (components.scheme == "https" && port == 443) { | ||
| components.port = nil | ||
| } | ||
|
|
||
| if components.path.count > 1, components.path.hasSuffix("/") { | ||
| components.path.removeLast() | ||
| } |
| /// Current step of the sign-in flow. `nil` means the user is not actively | ||
| /// signing in — the view should show the saved-servers list. |
| enum IntegrationViewMode { | ||
| /// Bound to a live library session; pre-populates form from the active connection. | ||
| case regular | ||
| /// Cog → Connection Details flow; pre-populates form, shows saved-list view. | ||
| case viewDetails | ||
| /// Dedicated Add Server flow; starts with empty form, no active-connection state leaks. | ||
| case addServer |
| /// Drives what the view renders. | ||
| /// `.enteringServerURL` → URL form; `.enteringCredentials` → credentials form; `nil` → saved-servers list. | ||
| var signInFlow: SignInStep? { get set } |
| case .none: | ||
| // Not in sign-in flow → render the saved-servers list. | ||
| IntegrationServerInformationSectionView( | ||
| serverName: viewModel.form.serverName, |
|
it's finally merged @matalvernaz 🎉 , sorry about the delay |
Brings in the now-upstream multi-server merge (TortugaPower#1491) and audio-session crash fix (TortugaPower#1524) plus the privacy policy bump. Conflict resolution: prefer upstream/develop where the same surface diverged, preserve test/all-changes work that develop doesn't have. - Multi-server core (10 Jellyfin/ABS/MainView files): took develop's reworked version. Matt's original multi-server commits are now effectively encoded in develop's squashed merge of TortugaPower#1491. - 26 Localizable.strings: took develop's base, re-appended local-only keys (Hummingbird/NNELS UI, Jellyfin Quick Connect, share-import, audio-session description) so referenced strings still resolve. - PlayerManager.swift: union of audio-session var declarations (both approaches reference symbols elsewhere in the file); took develop's Sentry-only recovery in the activation-failure block. Kept FLAC seek tracking (initialSeekInProgress / loadGeneration). - URL+BookPlayer.swift: took develop's loop trailing-slash strip. - project.pbxproj: union for the build/file ref blocks; deduped the orphaned A1B2.../F1A2B3C4... MediaServersView UUID entries that would have collided with develop's D5E6F708 UUIDs. Known carry-over from develop: JellyfinRootView.swift:608 still uses the raw key `"settings_title"` for accessibilityLabel rather than `.localized` — VoiceOver will read out the key name. Not patching here since it's an upstream bug; re-flag to @GianniCarlo.
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