From 5c7e659116caed246bffa3742ab36b547e6fd1a4 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 30 May 2026 01:09:07 -0400 Subject: [PATCH] refactor(android): hoist resolveServerImageUrl to shared/ServerUrls.kt as resolveServerUrl It was never image-specific -- it resolves any /api/* relative URL to the placeholder.invalid form that BaseUrlInterceptor rewrites. Both covers (ServerImage) and stream URLs (PlayerController) call it. Move the helper to shared/ServerUrls.kt with the right name; ServerImage and PlayerController import from there. Pure rename + relocate, no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../minstrel/player/PlayerController.kt | 7 +++-- .../fabledsword/minstrel/shared/ServerUrls.kt | 26 ++++++++++++++++++ .../minstrel/shared/widgets/ServerImage.kt | 27 ++----------------- 3 files changed, 31 insertions(+), 29 deletions(-) create mode 100644 android/app/src/main/java/com/fabledsword/minstrel/shared/ServerUrls.kt diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt index c9523274..6a172a0d 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt @@ -10,7 +10,7 @@ import androidx.media3.session.MediaController import androidx.media3.session.SessionToken import com.fabledsword.minstrel.di.ApplicationScope import com.fabledsword.minstrel.models.TrackRef -import com.fabledsword.minstrel.shared.widgets.resolveServerImageUrl +import com.fabledsword.minstrel.shared.resolveServerUrl import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.channels.Channel @@ -276,9 +276,8 @@ class PlayerController @Inject constructor( // Server's stream_url is a relative path (/api/tracks/{id}/stream); // resolve to the placeholder.invalid form so OkHttpDataSource (sharing // BaseUrlInterceptor) rewrites it to the live server with the auth - // cookie. Same resolution as covers — naming "ServerImage" is - // misleading and flagged for DRY follow-up. - val resolvedUri = resolveServerImageUrl(streamUrl) ?: streamUrl + // cookie. Same resolver as covers — one place for /api/* URLs. + val resolvedUri = resolveServerUrl(streamUrl) ?: streamUrl return MediaItem.Builder() .setMediaId(id) .setUri(resolvedUri) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/shared/ServerUrls.kt b/android/app/src/main/java/com/fabledsword/minstrel/shared/ServerUrls.kt new file mode 100644 index 00000000..1caeb7b1 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/shared/ServerUrls.kt @@ -0,0 +1,26 @@ +package com.fabledsword.minstrel.shared + +/** + * Resolves a relative server URL into something a Coil / OkHttp client + * can actually load. + * + * The server returns relative paths (e.g. `/api/albums/{id}/cover`, + * `/api/tracks/{id}/stream`) — host-less URIs that `OkHttpDataSource` + * and Coil's `AsyncImage` can't resolve on their own. Prepending the + * `placeholder.invalid` host lets `BaseUrlInterceptor` rewrite the + * request to the live server with the auth cookie. Absolute `http(s)` + * URLs pass through unchanged; blank input returns null so callers + * render their own fallback (or, for playback, the queue entry fails + * fast rather than handing Media3 a malformed URI). + * + * This is the single place a server URL becomes loadable, mirroring + * Flutter's `ServerImage._resolve` (which prepends the base URL). + * Any code handing a server-provided URL to a network client must + * route through here — covers, stream URLs, anything `/api/...`. + */ +fun resolveServerUrl(raw: String?): String? = when { + raw.isNullOrBlank() -> null + raw.startsWith("http://") || raw.startsWith("https://") -> raw + raw.startsWith("/") -> "http://placeholder.invalid$raw" + else -> "http://placeholder.invalid/$raw" +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/ServerImage.kt b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/ServerImage.kt index 80c09369..f30b9732 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/ServerImage.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/ServerImage.kt @@ -4,30 +4,7 @@ import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.layout.ContentScale import coil3.compose.AsyncImage - -/** - * Resolves a server image/cover URL into something Coil can load. - * - * The server returns **relative** cover URLs (e.g. - * `/api/albums/{id}/cover`, `/api/playlists/{id}/cover`). Coil can't - * load a host-less path, so we prefix the `placeholder.invalid` host - * that `BaseUrlInterceptor` rewrites to the live server (carrying the - * session cookie). Absolute `http(s)` URLs — including ones already - * built against the placeholder host — pass through unchanged; blank - * input returns null so the caller renders its own fallback. - * - * This is the single place a server URL becomes loadable, mirroring - * Flutter's `ServerImage._resolve` (which prepends the base URL). - * Cover surfaces must route through [ServerImage] rather than handing - * a raw `coverUrl` to `AsyncImage` — that was the bug where relative - * URLs from fresh fetches silently failed to load. - */ -fun resolveServerImageUrl(raw: String?): String? = when { - raw.isNullOrBlank() -> null - raw.startsWith("http://") || raw.startsWith("https://") -> raw - raw.startsWith("/") -> "http://placeholder.invalid$raw" - else -> "http://placeholder.invalid/$raw" -} +import com.fabledsword.minstrel.shared.resolveServerUrl /** * Renders a server-hosted image, resolving relative URLs centrally so @@ -42,7 +19,7 @@ fun ServerImage( contentScale: ContentScale = ContentScale.Crop, fallback: @Composable () -> Unit = {}, ) { - val resolved = resolveServerImageUrl(url) + val resolved = resolveServerUrl(url) if (resolved == null) { fallback() } else {